Skip to content

[Backport 4.5-7.16] Fix cannot read null properties bug in settings section#5216

Merged
yenienserrano merged 3 commits into
4.5-7.16from
backport-5135-to-4.5-7.16
Mar 10, 2023
Merged

[Backport 4.5-7.16] Fix cannot read null properties bug in settings section#5216
yenienserrano merged 3 commits into
4.5-7.16from
backport-5135-to-4.5-7.16

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented Feb 17, 2023

Backport 4c113f7 from #5135.

@chantal-kelm @github-actions
Fix cannot read null properties bug in settings section (#5135)
d2a7fbb 
* fixed bug cannot read null properties in settings section

* changelog

(cherry picked from commit 4c113f7)
@github-actions github-actions Bot requested a review from a team February 17, 2023 16:29
@Desvelao Desvelao linked an issue Feb 20, 2023 that may be closed by this pull request
Cannot read properties of null error in the Settings section #4772
Closed
@Desvelao
Copy link
Copy Markdown
Member

Desvelao commented Feb 20, 2023 •
edited
Loading

Kibana 7.16.3

Legend:
⚫: none
🟢: pass
🟡: warning
🔴: fail
⚪: not applicable

UI

Test Chrome Firefox Safari
Error is not displayed when going from Settings/Configuration to another section 🟢 🟢 ⚫
Platform loading indicator is displayed when saving the configuration in Settings/Configuration 🟢 ⚫ ⚫

Details

🟢 Error is not displayed when going from Settings/Configuration to another section

Chrome - 🟢
image

Firefox - 🟢
image

Safari - ⚫

🟢 Platform loading indicator is displayed when saving the configuration in Settings/Configuration

Chrome - 🟢
After changing the plugin configuration in Settings/Configuration, the loading in hte header of the plugin platform didn't seem to work.

I was researching this and seems they added a delay time to prevent the uneccessary loadings. Pull request: elastic/kibana#78879

I tried to increase the time to updating the plugin configuration through Settings/Configuration and the loading indicator was displayed. So, it seems to be working as expected.

Moreover, they did some changes in the loading indicator behavior from Kibana 7.10.2 to Kibana 7.16
Kibana 7.10.2 https://github.com/elastic/kibana/blob/v7.10.2/src/core/public/chrome/ui/loading_indicator.tsx#L43-L47
Kibnaa 7.16.3 https://github.com/elastic/kibana/blob/v7.16.3/src/core/public/chrome/ui/loading_indicator.tsx#L35-L45
Kibana 7.17.8 https://github.com/elastic/kibana/blob/v7.17.8/src/core/public/chrome/ui/loading_indicator.tsx#L35-L45

After applying a delay in the time to change the loading indicator status:
image

For another hand, this feature is only presentational and I think it is not a desired functionality. This could be removed in the future if we agree.

Firefox - ⚫

Safari - ⚫

Desvelao
Desvelao approved these changes Feb 20, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@Desvelao Desvelao left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review

🟢 code
🟢 test #5216 (comment)

@Desvelao
Copy link
Copy Markdown
Member

Desvelao commented Feb 20, 2023

The loading indicator when saving the plugin configuration through Settings/Configuration could not be displayed if the time of updating the plugin configuration is lower than 250ms. It is due to in change included in the plugin platform. See the test details in #5216 (comment) for more informaiton.

@github-actions
Copy link
Copy Markdown
Contributor Author

github-actions Bot commented Mar 9, 2023

Code coverage (Jest) % values
Statements 8.65% ( 3275 / 37870 )
Branches 4.44% ( 1300 / 29279 )
Functions 7.51% ( 706 / 9405 )
Lines 8.72% ( 3158 / 36219 )

@yenienserrano
Copy link
Copy Markdown
Member

yenienserrano commented Mar 10, 2023 •
edited
Loading

Kibana 7.16.3

Legend:
⚫: none
🟢: pass
🟡: warning
🔴: fail
⚪: not applicable

UI

Test Chrome Firefox Safari
Error is not displayed when going from Settings/Configuration to another section 🟢 🟢 ⚫
Platform loading indicator is displayed when saving the configuration in Settings/Configuration 🟢 🟢 ⚫

Details

🟢 Error is not displayed when going from Settings/Configuration to another section

Chrome - 🟢

image

Firefox - 🟢

image

Safari - ⚫

🟢 Platform loading indicator is displayed when saving the configuration in Settings/Configuration

Chrome - 🟢

image

Firefox - ⚫

image

Safari - ⚫

yenienserrano
yenienserrano approved these changes Mar 10, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@yenienserrano yenienserrano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

#5216 (comment)

@yenienserrano yenienserrano merged commit 63a58f9 into 4.5-7.16 Mar 10, 2023
@yenienserrano yenienserrano deleted the backport-5135-to-4.5-7.16 branch March 10, 2023 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Desvelao Desvelao Desvelao approved these changes

@yenienserrano yenienserrano yenienserrano approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cannot read properties of null error in the Settings section

3 participants

@Desvelao @yenienserrano @chantal-kelm